Skip to content

Done solution#296

Open
Andruwa808 wants to merge 4 commits into
mate-academy:masterfrom
Andruwa808:hm-9
Open

Done solution#296
Andruwa808 wants to merge 4 commits into
mate-academy:masterfrom
Andruwa808:hm-9

Conversation

@Andruwa808
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@pustoj0 pustoj0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit name must be about work you had done like implemented test, created calculator, WorkDone is a bad example. 👍

Comment thread pom.xml Outdated
Comment on lines +34 to +39
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.12</version>
<scope>test</scope>
</dependency>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dob't remember adding this dependecy, and it still worked ok with org.junit.jupiter.api.Assertions


public class CalculatorTest {
private static final double DELTA = 0.000001;
private static Calculator calc;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calculator calculator

}

@Test
public void positiveAddition() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong test methods names, read README.md more carefully. For this task use such convention: ; For example, if we are testing the method calculate with an illegal character passed as an operation the test method name should be calculate_illegalOperation_notOk. notOk is because the test expects the calculate method to throw an exception.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the same to everything.

@Test
public void positiveAddition() {
expected = 8;
actual = calc.calculator(1, 7, '+');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create private static variable and use them instead of raw numbers.

@Test
public void positiveDivision() {
expected = 2;
actual = calc.calculator(4,2, '/');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calc.calculator(4, 2, '/');
missed whitespace 😈

}

@Test
public void raisingPositiveToPositiveOrNegative() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raisingToPower, but I doubt it critical

@Test
public void negativeDivision() {
expected = 2;
actual = calc.calculator(-4,-2, '/');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😈

}

@Test
public void raisingZeroToPower() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was commented that it must be tested with positive and negative.

package core.basesyntax;

public class Calculator {
public double calculator(double firstNumber, double secondNumber, char operation) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public double calculator(double firstNumber, double secondNumber, char operation) {
public double calculate(double firstNumber, double secondNumber, char operation) {

@Test
public void calculator_raisingZeroToNegativePower_Ok() {
expected = 0;
actual = calculator.calculator(0,2,'^');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it negative?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants